-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor installation of wasmtime/wasm-tools on CI #387
Refactor installation of wasmtime/wasm-tools on CI #387
Conversation
We've got official actions for installation Wasmtime and wasm-tools in the bytecodealliance organization which should help make this installation step a bit more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seem like this change also updates the version of wasm-tools and wasmtime and the adapter?
If that desirable/indented can you mention the version bumps (old and and new version numbers) in the PR descrption?
.github/workflows/main.yml
Outdated
tar xf wasm-tools-1.0.54-x86_64-macos.tar.gz | ||
cp wasm-tools-1.0.54-x86_64-macos/wasm-tools ~/.wasmtime/bin/ | ||
fi | ||
- name: Setup `wasmtime` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe keep the "for tests" part of the old title, or include it in a comment so its clear that these things are only used for testing and not part of wasi-sdk itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, updated.
I can switch back to the old versions if desired, but it seems like they were arbitrarily chosen, so I took this opportunity to update them to the latest version of releases. They additionally no longer use |
sgtm, maybe just mention that in the PR description when this lands? |
Sure, I've updated the description. |
(I'll note I don't have write permissions on this repo so I'll need someone else to hit merge, also sorry if this is already known, I'm not sure what the desired etiquette is on this repo for merging PRs) |
@alexcrichton I added you as a contributor |
Oh awesome, thank you! |
We've got official actions for installation Wasmtime and wasm-tools in the bytecodealliance organization which should help make this installation step a bit more readable.
This additionally updates a few versions:
dev
tag is no longer needed.dev
is no longer needed.